Fix: Deterministic variables in sample_posterior_predictive no longer cause resampling #7969
+437
−10
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix:
sample_posterior_predictiveincorrectly resamples dependencies of Deterministic variablesProblem Description
When using
sample_posterior_predictivewithpredictions=Trueand including aDeterministicvariable invar_names, the function incorrectly resamples the Deterministic's dependencies (random variables) instead of using their values from the posterior trace.Expected Behavior
When a
Deterministicvariable is included invar_names, it should be recomputed from the posterior samples of its dependencies, not cause those dependencies to be resampled.Actual Behavior (Before Fix)
The dependencies (random variables) were resampled (likely from their priors), causing:
Impact
This bug affects any use case where:
var_namesforsample_posterior_predictiveRoot Cause
In
pymc/sampling/forward.py, thecompile_forward_sampling_functionmarks variables as "volatile" if they:When a
Deterministicvariable was included invar_names:intercepts,slopes, etc.) got incorrectly marked as volatileThe documentation previously acknowledged this problem (line 844 in
forward.py):Solution
Added a second pass in
compile_forward_sampling_functionthat:Implementation
The fix adds a second pass after the initial volatile node marking:
This ensures that when
sample_posterior_predictiveis called withvar_names=["mu_grid"](wheremu_gridis a Deterministic), the output showsSampling: []instead ofSampling: [intercepts, slopes], indicating that no variables are being resampled.Changes Made
Fixed
compile_forward_sampling_functioninpymc/sampling/forward.py:deterministicsparameter to identify Deterministic variablesmodel.deterministicsAdded comprehensive test suite in
tests/sampling/test_deterministic_posterior_predictive.py:Updated documentation in
pymc/sampling/forward.py:Testing
All tests pass, including:
Example
Question for Maintainers
Does this fix align with the intended behavior of
sample_posterior_predictive? We believe this is the correct behavior - Deterministic variables should be recomputed from trace values, not cause their dependencies to be resampled.